notebook tabs shows first uncommon subdirectory if filename is the same #365

zalox · 2018-02-11T16:45:05Z

No description provided.

eidheim · 2018-02-12T09:42:49Z

I had a look at this, and there were some minor issues:

The labels got the /.../-prefix even though only the filenames were equal
When closing a tab with a path prefix, the tabs did not get updated.
update_tab_label should be called when opening a new buffer, just in case the buffer is empty and on_change is never called.
Maybe slightly more complex code than needed.

I did a fix proposal here that you can look at:

diff --git a/src/notebook.cc b/src/notebook.cc
index 0cbdf75..4f4c654 100644
--- a/src/notebook.cc
+++ b/src/notebook.cc
@@ -10,14 +10,6 @@
 #include "source_language_protocol.h"
 #include "gtksourceview-3.0/gtksourceview/gtksourcemap.h"
 
-template <class firstIt, class secondIt>
-  auto mismatch(firstIt first1, firstIt last1, secondIt first2,secondIt last2){
-  while (first1 != last1 && first2 != last2 && *first1 == *first2) {
-      ++first1, ++first2;
-  }
-  return std::make_pair(*first1, *first2);
-};
-
 Notebook::TabLabel::TabLabel(const boost::filesystem::path &path, std::function<void()> on_close) {
   set_can_focus(false);
   set_tooltip_text(path.string());
@@ -26,7 +18,6 @@ Notebook::TabLabel::TabLabel(const boost::filesystem::path &path, std::function<
   auto hbox=Gtk::manage(new Gtk::Box());
   
   hbox->set_can_focus(false);
-  label.set_text(path.filename().string()+' ');
   label.set_can_focus(false);
   button->set_image_from_icon_name("window-close-symbolic", Gtk::ICON_SIZE_MENU);
   button->set_can_focus(false);
@@ -203,12 +194,10 @@ void Notebook::open(const boost::filesystem::path &file_path_, size_t notebook_i
     }
   };
   source_views.back()->update_tab_label=[this](Source::View *view) {
-    const auto update_label = [&](const boost::filesystem::path &path, size_t index){
+    const auto update_label = [this](size_t index, const std::string &prepend) {
       const auto current_view = source_views[index];
-      std::string title = current_view->file_path.filename().string();
-      if(!path.empty())
-        title = path.string() + "/.../" + title;
-      if(view->get_buffer()->get_modified())
+      auto title = prepend+current_view->file_path.filename().string();
+      if(current_view->get_buffer()->get_modified())
         title+='*';
       else
         title+=' ';
@@ -216,32 +205,32 @@ void Notebook::open(const boost::filesystem::path &file_path_, size_t notebook_i
       tab_label->label.set_text(title);
       tab_label->set_tooltip_text(filesystem::get_short_path(current_view->file_path).string());
     };
-    std::unordered_map<int, std::pair<boost::filesystem::path, boost::filesystem::path>> uncommon_paths;
     const auto file_name=view->file_path.filename();
     size_t current_view_index = 0;
+    std::string prepend_current_view;
     for(size_t c=0;c<size();++c) {
       if(source_views[c]==view) {
         current_view_index = c;
         continue;
       }
       if (source_views[c]->file_path.filename() == file_name) {
-        auto const first_uncommon_path = mismatch(
-          view->file_path.parent_path().rbegin(),
-          view->file_path.parent_path().rend(),
-          source_views[c]->file_path.parent_path().rbegin(),
-          source_views[c]->file_path.parent_path().rend()
-        );
-        uncommon_paths.emplace(std::make_pair(c, first_uncommon_path));
+        int diff=0;
+        auto parent_path1=view->file_path.parent_path();
+        auto parent_path2=source_views[c]->file_path.parent_path();
+        auto it1=parent_path1.rbegin();
+        auto it2=parent_path2.rbegin();
+        while (it1 != parent_path1.rend() &&
+               it2 != parent_path2.rend() && *it1 == *it2) {
+          ++it1;
+          ++it2;
+          ++diff;
+        }
+        if(prepend_current_view.empty())
+          prepend_current_view=it1->string()+(diff>0?"/.../":"/");
+        update_label(c, it2->string()+(diff>0?"/.../":"/"));
       }
     }
-    for (const auto &uncommon_path : uncommon_paths) {
-      update_label(uncommon_path.second.second, uncommon_path.first);
-    }
-    if (uncommon_paths.empty()) {
-      update_label("", current_view_index);
-    } else {
-      update_label(uncommon_paths.begin()->second.first, current_view_index);
-    }
+    update_label(current_view_index, prepend_current_view);
     update_status(view);
   };
   source_views.back()->update_status_diagnostics=[this](Source::View* view) {
@@ -327,6 +316,8 @@ void Notebook::open(const boost::filesystem::path &file_path_, size_t notebook_i
     if(index!=static_cast<size_t>(-1))
       close(index);
   }));
+  if(source_views.back()->update_tab_label)
+    source_views.back()->update_tab_label(source_views.back());
   
   //Add star on tab label when the page is not saved:
   source_view->get_buffer()->signal_modified_changed().connect([this, source_view]() {
@@ -540,6 +531,11 @@ bool Notebook::close(size_t index) {
     hboxes.erase(hboxes.begin()+index);
     tab_labels.erase(tab_labels.begin()+index);
   }
+  
+  for(auto view: get_views()) { // Update all view tabs in case one clicks cross to close a buffer
+    if(view->update_tab_label)
+      view->update_tab_label(view);
+  }
   return true;
 }

eidheim · 2018-02-14T11:52:15Z

src/notebook.cc

-        tab_label->set_tooltip_text(filesystem::get_short_path(view->file_path).string());
-        update_status(view);
-        return;
+        update_label(c, prepend_current_view);


prepend_current_view might not have been set here yet.

eidheim · 2018-02-14T11:53:43Z

src/notebook.cc

+        while (it1 != parent_path1.rend() && it2 != parent_path2.rend() && *it1 == *it2) {
+                  ++it1;
+                  ++it2;
+                  ++diff;


long indentation, also feel free to keep these at one line like you did before. I changed it by accident to multiple lines while testing.

eidheim · 2018-02-24T09:37:50Z

Here are some suggestion for changes that also fixes double / when root path is shown:

diff --git a/src/notebook.cc b/src/notebook.cc
index c4dd4c1..4d7c5ef 100644
--- a/src/notebook.cc
+++ b/src/notebook.cc
@@ -209,6 +209,7 @@ void Notebook::open(const boost::filesystem::path &file_path_, size_t notebook_i
     const auto file_name = view->file_path.filename();
     std::string prepend_current_view;
     size_t current_view_index = 0;
+    // Extends tab label with a parent folder if two tab labels show the same filename
     for (size_t c = 0; c < size(); ++c) {
       if (source_views[c] == view) {
         current_view_index = c;
@@ -218,16 +219,26 @@ void Notebook::open(const boost::filesystem::path &file_path_, size_t notebook_i
         int diff = 0;
         const auto parent_path1 = view->file_path.parent_path();
         const auto parent_path2 = source_views[c]->file_path.parent_path();
-        auto it1 = parent_path1.rbegin();
-        auto it2 = parent_path2.rbegin();
-        while (it1 != parent_path1.rend() && it2 != parent_path2.rend() && *it1 == *it2) {
+        // Ubuntu 16.04 workaround:
+        auto it1 = std::reverse_iterator<boost::filesystem::path::iterator>(parent_path1.end());
+        auto it1_end = std::reverse_iterator<boost::filesystem::path::iterator>(parent_path1.begin());
+        auto it2 = std::reverse_iterator<boost::filesystem::path::iterator>(parent_path2.end());
+        auto it2_end = std::reverse_iterator<boost::filesystem::path::iterator>(parent_path2.begin());
+        while (it1 != it1_end && it2 != it2_end && *it1 == *it2) {
           ++it1;
           ++it2;
           ++diff;
         }
-        if (prepend_current_view.empty())
-          prepend_current_view = it1->string() + (diff > 0 ? "/.../" : "/");
-        update_label(c, it2->string() + (diff > 0 ? "/.../" : "/"));
+        if (prepend_current_view.empty()) {
+          auto it1_str = it1->string();
+          if(it1_str=="/") // Fix for root directory
+            it1_str="";
+          prepend_current_view = it1_str + (diff > 0 ? "/.../" : "/");
+        }
+        auto it2_str = it2->string();
+        if(it2_str=="/") // Fix for root directory
+          it2_str="";
+        update_label(c, it2_str + (diff > 0 ? "/.../" : "/"));
       }
     }
     update_label(current_view_index, prepend_current_view);

eidheim · 2018-02-24T09:44:46Z

One problem though is that if you have a directory structure like this:

a/
- b/
  - 1.txt
- 1.txt
b/
- 1.txt

and you open all the 1.txt files, then if you modify them, their labels will change depending on which file you modify.

zalox · 2018-02-28T12:31:45Z

I will have time to look at this this today or this weekend. Feel free to fix it.

zalox force-pushed the notebook_tabs branch 2 times, most recently from 7e962d4 to 3420b50 Compare February 13, 2018 22:18

eidheim reviewed Feb 14, 2018

View reviewed changes

zalox force-pushed the notebook_tabs branch from 80786b5 to c265ead Compare March 7, 2018 14:19

zalox added 4 commits April 3, 2018 18:06

notebook tabs shows first uncommon subdirectory if filename is the same

58caac1

update on open and close with x

adb56bc

fix prepend on current index, also format

a7c2b33

only add star on update

8534ee2

zalox force-pushed the notebook_tabs branch from c265ead to 8534ee2 Compare April 4, 2018 16:10

zalox added 2 commits April 5, 2018 15:47

fixes for ubuntu 16

df0b0a5

use epsilon char

3530f3c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

notebook tabs shows first uncommon subdirectory if filename is the same #365

notebook tabs shows first uncommon subdirectory if filename is the same #365

zalox commented Feb 11, 2018

eidheim commented Feb 12, 2018

eidheim Feb 14, 2018

eidheim Feb 14, 2018

eidheim commented Feb 24, 2018

eidheim commented Feb 24, 2018

zalox commented Feb 28, 2018

notebook tabs shows first uncommon subdirectory if filename is the same #365

Are you sure you want to change the base?

notebook tabs shows first uncommon subdirectory if filename is the same #365

Conversation

zalox commented Feb 11, 2018

eidheim commented Feb 12, 2018

eidheim Feb 14, 2018

Choose a reason for hiding this comment

eidheim Feb 14, 2018

Choose a reason for hiding this comment

eidheim commented Feb 24, 2018

eidheim commented Feb 24, 2018

zalox commented Feb 28, 2018